Skip to content

Attempt at fixing LT-22708 - #1122

Open
AndyBlack wants to merge 4 commits into
mainfrom
LT22708
Open

Attempt at fixing LT-22708#1122
AndyBlack wants to merge 4 commits into
mainfrom
LT22708

Conversation

@AndyBlack

@AndyBlack AndyBlack commented Sep 3, 2026 •
edited by jasonleenaylor
Loading

Copy link
Copy Markdown
Collaborator

Change-Id: I24735bad6cd9e8823a0a028151afdc64722d5a8e
Using XAmple with additional properties can cause a crash per what is in https://jira.sil.org/browse/LT-22708.
This fix has the XAmple parser send a refresh message just after the new list is added. It does cause the Try a Word window to flash and then disappear while the main window also flashes but returns.

I do not know what is causing the Try a Word window to disappear although I suspect it may have to do with the parser running in a separate thread...

This change is Reviewable

@AndyBlack
Attempt at fixing LT-22708
259bd88 
Change-Id: I24735bad6cd9e8823a0a028151afdc64722d5a8e
@github-actions

github-actions Bot commented Sep 3, 2026 •
edited
Loading

Copy link
Copy Markdown

Comment hygiene (advisory)

No comment-style violations in the lines this branch adds since origin/main.

@github-actions

github-actions Bot commented Sep 3, 2026 •
edited
Loading

Copy link
Copy Markdown

NUnit Tests

    1 files  ± 0      1 suites  ±0   9m 26s ⏱️ - 2m 13s
6 084 tests +73  5 999 ✅ +69  85 💤 +4  0 ❌ ±0 
6 093 runs  +73  6 008 ✅ +69  85 💤 +4  0 ❌ ±0 

Results for commit 62ffe98. ± Comparison against base commit 1ab408b.

This pull request removes 2 and adds 75 tests. Note that renamed tests count towards both. 
SIL.FieldWorks.FwCoreDlgControlsTests.TestFontFeaturesButton ‑ OpenTypeFontFeatureReader_CachesFeatureTagsForSameFontKey
SIL.FieldWorks.FwCoreDlgControlsTests.TestFontFeaturesButton ‑ OpenTypeFontFeatureReader_FiltersRequiredShapingFeatures

SIL.FieldWorks.Common.FwUtils.FlexBridgeListenerTests ‑ FlexBridgeDataVersion
SIL.FieldWorks.Common.FwUtils.FwDirectoryFinderTests ‑ CodeAndDataDirectory_PreferSourceTreeOverRegistry("RootCodeDir")
SIL.FieldWorks.Common.FwUtils.FwDirectoryFinderTests ‑ CodeAndDataDirectory_PreferSourceTreeOverRegistry("RootDataDir")
SIL.FieldWorks.Common.FwUtils.FwDirectoryFinderTests ‑ FindDevDistFiles_InsideSourceTree_FindsTreeDistFiles("Output/Debug")
SIL.FieldWorks.Common.FwUtils.FwDirectoryFinderTests ‑ FindDevDistFiles_InsideSourceTree_FindsTreeDistFiles("Output/Debug/x64")
SIL.FieldWorks.Common.FwUtils.FwDirectoryFinderTests ‑ FindDevDistFiles_InsideSourceTree_FindsTreeDistFiles("Src/Common/FwUtils/bin/Debug/net8.0")
SIL.FieldWorks.Common.FwUtils.FwDirectoryFinderTests ‑ FindDevDistFiles_OutsideSourceTree_ReturnsNull
SIL.FieldWorks.Common.FwUtils.FwDirectoryFinderTests ‑ FindDevDistFiles_UnderAPathContainingASpace_FindsTreeDistFiles
SIL.FieldWorks.Common.FwUtils.FwDirectoryFinderTests ‑ FindSourceDirectory_FromASupportedLayout_FindsTreeSrc("Output","Debug")
SIL.FieldWorks.Common.FwUtils.FwDirectoryFinderTests ‑ FindSourceDirectory_FromASupportedLayout_FindsTreeSrc("Output","Debug","x64")
…

♻️ This comment has been updated with latest results.

@codecov-commenter

codecov-commenter commented Sep 3, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 14.28571% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.63%. Comparing base (6f72671) to head (62ffe98).
⚠️ Report is 30 commits behind head on main.

Files with missing lines Patch % Lines
Src/LexText/ParserCore/XAmplePropertiesPreparer.cs 0.00% 8 Missing and 3 partials ⚠️
Src/LexText/ParserCore/XAmpleParser.cs 28.57% 4 Missing and 1 partial ⚠️
...ies/pcpatrflex/ToneParsFLExDll/ToneParsFLExForm.cs 0.00% 2 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1122      +/-   ##
==========================================
+ Coverage   38.33%   38.63%   +0.29%     
==========================================
  Files        1507     1516       +9     
  Lines      350580   351401     +821     
  Branches    40293    40421     +128     
==========================================
+ Hits       134410   135760    +1350     
+ Misses     186941   186398     -543     
- Partials    29229    29243      +14
Files with missing lines Coverage Δ
Src/FwParatextLexiconPlugin/FdoLexicon.cs 59.24% <ø> (-3.36%) ⬇️
Src/LexText/ParserCore/ParserWorker.cs 64.07% <100.00%> (ø)
...ies/pcpatrflex/ToneParsFLExDll/ToneParsFLExForm.cs 0.00% <0.00%> (ø)
Src/LexText/ParserCore/XAmpleParser.cs 32.86% <28.57%> (-25.54%) ⬇️
Src/LexText/ParserCore/XAmplePropertiesPreparer.cs 0.00% <0.00%> (ø)

... and 93 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

papeh
papeh reviewed Sep 3, 2026
View reviewed changes
Comment thread Src/LexText/ParserCore/XAmplePropertiesPreparer.cs Outdated
Comment thread Src/LexText/ParserCore/XAmplePropertiesPreparer.cs
Comment thread Src/FwParatextLexiconPlugin/FdoLexicon.cs Outdated
Comment thread Src/LexText/ParserCore/XAmpleParser.cs
@AndyBlack
Improve clarity and fix comment length
bbe9293 
Change-Id: Ie7662021a72fe8e6f47d639d2ea4ea05dc54f3d9
@AndyBlack
Have FdoLexicon set parsers to null for now
2622039 
Change-Id: Icae61236636f4dc06d7ff20f13205e9b7fb0d016
papeh
papeh requested changes Sep 11, 2026
View reviewed changes
Comment thread Src/FwParatextLexiconPlugin/FdoLexicon.cs Outdated
Comment thread Src/FwParatextLexiconPlugin/FwParatextLexiconPluginTests/FdoLexiconTests.cs Outdated
@AndyBlack
Remove InstantiateParser()
62ffe98 
Change-Id: I661a918db017f75f44eac62a922e93165cd08afa
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasonleenaylor jasonleenaylor jasonleenaylor left review comments
@papeh papeh papeh requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@AndyBlack @codecov-commenter @jasonleenaylor @papeh